Skip to content

fix: doc comments regarding safety for Number.toU32()#27

Merged
spiral-ladder merged 1 commit into
mainfrom
bing/doc-com-fix
May 5, 2026
Merged

fix: doc comments regarding safety for Number.toU32()#27
spiral-ladder merged 1 commit into
mainfrom
bing/doc-com-fix

Conversation

@spiral-ladder
Copy link
Copy Markdown
Contributor

Discovered this while working on ChainSafe/lodestar-z#349.

napi_get_value_uint32 makes no guarantee of erroring on values out of bounds:

Let int32bit be int modulo 2**32.

Source: https://tc39.es/ecma262/#sec-touint32

This little code snippet PoC when added to BeaconStateView.test.ts in lodestar-z passes:

  it("toU32 silently wraps around", () => {
    let i = 0;
    for (i = 0; i < 100; i++) {
      const a = state.getBalance(0 + i);
      const b = state.getBalance(2 ** 32 + i);
      expect(a).toBe(b);    
      }
  });

While this fails, but only because we're access the SSZ list index out of bounds:

state.getBalance(-1)
 FAIL  bindings/test/beaconStateView.test.ts > BeaconStateView > probe truncation behavior
Error: IndexOutOfBounds

Other conversion methods might behave the same; i haven't fully checked, but we should be careful of this

Discovered this while working on ChainSafe/lodestar-z#349.

`napi_get_value_uint32` makes no guarantee of erroring on values out of
bounds:

> Let int32bit be int modulo 2**32.

Source: https://tc39.es/ecma262/#sec-touint32

This little code snippet PoC when added to `BeaconStateView.test.ts` in
lodestar-z passes:

```ts
  it("toU32 silently wraps around", () => {
    let i = 0;
    for (i = 0; i < 100; i++) {
      const a = state.getBalance(0 + i);
      const b = state.getBalance(2 ** 32 + i);
      expect(a).toBe(b);    
      }
  });
```

While this fails, but only because we're access the SSZ list index out of
bounds:

```ts
state.getBalance(-1)
```

```
 FAIL  bindings/test/beaconStateView.test.ts > BeaconStateView > probe truncation behavior
Error: IndexOutOfBounds
```

Other conversion methods might behave the same; i haven't fully checked,
but we should be careful of this
@spiral-ladder spiral-ladder requested a review from nazarhussain May 5, 2026 11:34
@spiral-ladder spiral-ladder self-assigned this May 5, 2026
@spiral-ladder spiral-ladder merged commit 4e74585 into main May 5, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants